Repository navigation
fix(node,vercel-edge,bundler-plugins): INC-2566 - Ignore commit env vars set to their own name - #25244
fix(node,vercel-edge,bundler-plugins): INC-2566 - Ignore commit env vars set to their own name#25244mwarkentin wants to merge 4 commits into
Conversation
…their own name Some builds have `VERCEL_GIT_COMMIT_SHA` set to the literal string `VERCEL_GIT_COMMIT_SHA` instead of a SHA. Release auto-detection took that value as the release name, so every build of the app uploaded its source maps into, and reported events against, one release that never changes. Release detection in the bundler plugins and the Node and Vercel Edge SDKs now skips a commit env var whose value is its own name, warns, and moves on to the next candidate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WgDm3ijhQZQUHGAFFqQzKQ
size-limit report 📦
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 67bead4. Configure here.
…mmit vars that would be the release Read the release candidate groups lazily, so a self-named commit variable is warned about only when no earlier variable already supplied the release. Drop the second CI_COMMIT_SHA read for Woodpecker CI: the GitLab check already reads that variable, so the second read could only repeat the warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WgDm3ijhQZQUHGAFFqQzKQ
isaacs
left a comment
isaacs
left a comment
There was a problem hiding this comment.
The changes here mostly look fine, but there's two main concerns to address before landing this, imo.
The first is that it should link to an issue, or build, or something, where this actually happened, so that it's not a mystery if we go digging for the source someday. If it's just a theoretical failure, then that's fine, but it should say so in the PR description.
Second issue, packages/bundler-plugins/src/core/options-mapping.ts has this on lines 137-142:
if (options.release.setCommits === undefined) {
if (
process.env['VERCEL'] &&
process.env['VERCEL_GIT_COMMIT_SHA'] &&
process.env['VERCEL_GIT_REPO_SLUG'] &&
process.env['VERCEL_GIT_REPO_OWNER'] &&which seems like it's potentially the same failure that this is seeking to address?
I'd recommend exporting the readCommitEnvVar from utils.ts and use it in the setCommits condition and value, so a placeholder SHA also skips automatic commit association.
Follow-up, for a future PR, out of scope here: The release detection list now exists in three copies (packages/node/src/sdk/api.ts, packages/vercel-edge/src/sdk.ts, packages/bundler-plugins/src/core/utils.ts), and they already differ (Node and Vercel Edge read HEROKU_BUILD_COMMIT; the bundler plugins do not). This PR adds a fourth and fifth function (readCommitEnvVar) that have to stay in sync. Moving the list and the helper to one shared place would remove that drift.
…r commit SHA Read VERCEL_GIT_COMMIT_SHA and VERCEL_GIT_PREVIOUS_SHA through readCommitEnvVar when setting commits, so a variable set to its own name falls back to automatic commit association instead of being sent as the commit. Warn about each placeholder variable once per build, since release detection and commit association can both read it, and note why the warning bypasses the plugin logger. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WgDm3ijhQZQUHGAFFqQzKQ
|
Thanks @isaacs, both addressed:
Agreed on the shared release-detection list (including the Generated by Claude Code |
…day-nki4g5 # Conflicts: # packages/bundler-plugins/src/core/utils.ts
|
Generated by Claude Code |
mwarkentin
changed the title
This branch has not been deployed
3 participants
Some builds have
VERCEL_GIT_COMMIT_SHAset to the literal stringVERCEL_GIT_COMMIT_SHAinstead of a SHA. Release auto-detection took that value as the release name, so every build of the app uploaded its source maps into, and reported events against, one release that never changes.This is not theoretical: Sentry has received source map uploads and events whose release is the literal string
VERCEL_GIT_COMMIT_SHAfrom a number of organizations deploying on Vercel. Some were large enough to contribute to INC-2566. How those builds end up with that value is still being investigated; getsentry/sentry#127101 logs the placeholder name at upload time to help with that. This guard stops the value from becoming the release either way.Release detection in the bundler plugins and the Node and Vercel Edge SDKs now skips a commit env var whose value is its own name, warns, and moves on to the next candidate. Candidates are read in order and only until one is found, so a placeholder is only warned about when it would otherwise have become the release. In the bundler plugins, the Vercel commit association in
setCommitsuses the same check, so a placeholder SHA falls back to automatic commit association instead of being sent as the commit.Claude-Session: https://claude.ai/code/session_01WgDm3ijhQZQUHGAFFqQzKQ
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).